senpai: use SCFG format #4086

jleightcap · 2023-06-13T07:42:32Z

Description

Closes #2534.

Checklist

Change is backwards compatible.
- renamed config args to follow the upstream: https://git.sr.ht/~taiite/senpai/tree/master/item/doc/senpai.5.scd
- deprecate the no-tls option. upstream config now exposes just tls with a reasonable default.
Code formatted with ./format.
Code tested through nix-shell --pure tests -A run.all or nix develop --ignore-environment .#all using Flakes.
Test cases updated/added. See example.
Commit messages are formatted like
```
{component}: {description}

{long description}
```
See CONTRIBUTING for more information and recent commit messages for examples.
If this PR adds a new module
- Added myself as module maintainer. See example.

jleightcap · 2023-06-13T07:47:00Z

modules/programs/senpai.nix

-    xdg.configFile."senpai/senpai.yaml".source =
-      cfgFmt.generate "senpai.yaml" cfg.config;
+    xdg.configFile."senpai/senpai.scfg".text = let
+      toSCFG' = v:


This is an inline generator for the (pretty custom) SCFG format. Would a PR to add this to generators.nix be more appropriate?

Added in this PR.

modules/programs/senpai.nix

Fix NixOS/nixpkgs#148667 introducing the backwards-incompatible change of using the SCFG format (https://git.sr.ht/~emersion/scfg). Signed-off-by: Jack Leightcap <jack@leightcap.com>

ncfavier · 2023-06-13T08:40:03Z

I'd really rather keep the freeformType and add an SCFG generator. I wonder to what extent our toKDL generator produces valid SCFG.

ncfavier · 2023-06-13T08:40:51Z

The renamed options can have mkRenamedOptionModules for a smooth transition.

@malte-v

remove @malte-v: #4086 (comment) add jleightcap (commit author) Signed-off-by: Jack Leightcap <jack@leightcap.com>

ncfavier · 2023-06-13T08:54:14Z

tests/modules/programs/senpai/example-settings-expected.conf

+  nicknames 16
+}
+
+password-cmd gopass show irc/guest


Shouldn't this be password-cmd "gopass show irc/guest"?

No, SCFG doesn't quote its strings. See: https://git.sr.ht/~taiite/senpai/tree/master/item/doc/senpai.5.scd#L199

It does, but password-cmd is intentionally parsed as a list of arguments to be fed to an execve-like interface, rather than a single shell command string: https://git.sr.ht/~taiite/senpai/tree/master/item/config.go#L199-214

The option should reflect that by having type nullOr (listOf str). This will avoid giving users the illusion that they can use shell syntax and quoting.

Ah, I understand. Thank you for the well reasoned response -- an option wrapping execve as a list of strings is much more correct 🙂

ncfavier · 2023-06-13T08:59:11Z

As far as I can tell, toKDL works fine at least for the test configuration. We just need to filter out null values.

Maybe just add toSCFG = toKDL; in generators.nix for now?

modules/programs/senpai.nix

Signed-off-by: Jack Leightcap <jack@leightcap.com>

two prior config choices: - `addr` - `nick` are renamed to match expected keys from upstream. Signed-off-by: Jack Leightcap <jack@leightcap.com>

removed upstream. move into `extraConfig` with `tls` option. Signed-off-by: Jack Leightcap <jack@leightcap.com>

jleightcap · 2023-06-19T00:02:11Z

As far as I can tell, toKDL works fine at least for the test configuration. We just need to filter out null values.

Maybe just add toSCFG = toKDL; in generators.nix for now?

Unfortunately existing toKDL isn't close enough to SCFG, mostly related to string handling, but thanks for the pointer. Instead I've included a de novo generator here.

Let me know if more testing is warranted -- since senpai seems to be the only package in-tree that supports SCFG I didn't aim for a fully general generator.

Signed-off-by: Jack Leightcap <jack@leightcap.com>

modules/programs/senpai.nix

modules/lib/generators.nix

modules/programs/senpai.nix

Signed-off-by: jleightcap <jack@leightcap.com>

ncfavier · 2023-06-20T10:49:33Z

modules/programs/senpai.nix

+  imports = [
+    (mkRenamedOptionModule [ "programs" "senpai" "addr" ] [
+      "programs"
+      "senpai"
+      "address"
+    ])
+    (mkRenamedOptionModule [ "programs" "senpai" "nick" ] [
+      "programs"
+      "senpai"
+      "nickname"
+    ])
+    (mkChangedOptionModule [ "programs" "senpai" "no-tls" ] [
+      "programs"
+      "senpai"
+      "tls"
+    ] (config: !config.programs.senpai.no-tls))
+  ];


Sadly this is not going to work because of NixOS/nixpkgs#96006. (Well, and also because this is defined in a let in but not used)

I think the best we can do for now is add assertions that the old options aren't used...

Sorry for leading you to a wrong track, this corner of the module system isn't quite ready yet.

stale · 2023-09-21T22:42:04Z

Thank you for your contribution! I marked this pull request as stale due to inactivity. Please read the relevant sections below before commenting.

If you are the original author of the PR

GitHub sometimes doesn't notify people who commented / reviewed a PR previously when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
If it is unfinished but you plan to finish it, please mark it as a draft.
If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
To get things rolling again, rebase the PR against the target branch and address valid comments.

If you are not the original author of the PR

If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

jleightcap commented Jun 13, 2023

View reviewed changes

modules/programs/senpai.nix Outdated Show resolved Hide resolved

senpai: use SCFG format (#2534)

55f880f

Fix NixOS/nixpkgs#148667 introducing the backwards-incompatible change of using the SCFG format (https://git.sr.ht/~emersion/scfg). Signed-off-by: Jack Leightcap <jack@leightcap.com>

jleightcap marked this pull request as ready for review June 13, 2023 07:54

senpai: maintainers handoff

144ff00

remove @malte-v: #4086 (comment) add jleightcap (commit author) Signed-off-by: Jack Leightcap <jack@leightcap.com>

ncfavier reviewed Jun 13, 2023

View reviewed changes

modules/programs/senpai.nix Outdated Show resolved Hide resolved

jleightcap added 4 commits June 18, 2023 19:18

senpai: SCFG generator

41bb341

Signed-off-by: Jack Leightcap <jack@leightcap.com>

senpai: password assertions

ea7404e

Signed-off-by: Jack Leightcap <jack@leightcap.com>

senpai: renamed options

62da2b8

two prior config choices: - `addr` - `nick` are renamed to match expected keys from upstream. Signed-off-by: Jack Leightcap <jack@leightcap.com>

senpai: deprecate no-tls option

b59d90b

removed upstream. move into `extraConfig` with `tls` option. Signed-off-by: Jack Leightcap <jack@leightcap.com>

jleightcap added 4 commits June 19, 2023 15:28

senpai: use literal tags

14adb57

Signed-off-by: Jack Leightcap <jack@leightcap.com>

senpai: password-cmd use execve interface

8151418

Signed-off-by: Jack Leightcap <jack@leightcap.com>

fixup! senpai: SCFG generator

55336fb

fixup! senpai: use literal tags

a6a95f0

ncfavier reviewed Jun 19, 2023

View reviewed changes

senpai: pr feedback

5a861c9

Signed-off-by: jleightcap <jack@leightcap.com>

ncfavier reviewed Jun 20, 2023

View reviewed changes

stale bot added the status: stale label Sep 21, 2023

patwid mentioned this pull request Nov 12, 2023

senpai: switch to scfg format #4650

Merged

5 tasks

jleightcap closed this by deleting the head repository Jan 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

senpai: use SCFG format #4086

senpai: use SCFG format #4086

jleightcap commented Jun 13, 2023 •

edited

Loading

jleightcap Jun 13, 2023

jleightcap Jun 18, 2023

ncfavier commented Jun 13, 2023

ncfavier commented Jun 13, 2023

ncfavier Jun 13, 2023

jleightcap Jun 18, 2023

ncfavier Jun 19, 2023 •

edited

Loading

jleightcap Jun 19, 2023

ncfavier commented Jun 13, 2023

jleightcap commented Jun 19, 2023

ncfavier Jun 20, 2023

stale bot commented Sep 21, 2023

senpai: use SCFG format #4086

senpai: use SCFG format #4086

Conversation

jleightcap commented Jun 13, 2023 • edited Loading

Description

Checklist

jleightcap Jun 13, 2023

Choose a reason for hiding this comment

jleightcap Jun 18, 2023

Choose a reason for hiding this comment

ncfavier commented Jun 13, 2023

ncfavier commented Jun 13, 2023

ncfavier Jun 13, 2023

Choose a reason for hiding this comment

jleightcap Jun 18, 2023

Choose a reason for hiding this comment

ncfavier Jun 19, 2023 • edited Loading

Choose a reason for hiding this comment

jleightcap Jun 19, 2023

Choose a reason for hiding this comment

ncfavier commented Jun 13, 2023

jleightcap commented Jun 19, 2023

ncfavier Jun 20, 2023

Choose a reason for hiding this comment

stale bot commented Sep 21, 2023

jleightcap commented Jun 13, 2023 •

edited

Loading

ncfavier Jun 19, 2023 •

edited

Loading